feat(catalogs): artist_account_id filter + no-cap pagination on GET /api/catalogs/measurements#763
Conversation
…api/catalogs/measurements (chat#1850) Two fixes on the same read path (v2 of api#757): - FIX the silent 1,000-row cap: the catalog tracklist, the measurement series, and the artist-link read all paginate to exhaustion with .range() loops (and the measurements IN clause is chunked at 500 ISRCs to stay under URL limits) BEFORE dedupe/derivation, so total_streams and the valuation band cover every measured song. Live repro was a 2,679-song catalog valued 3.4x low off exactly 1,000 rows. - ADD optional artist_account_id (uuid, 400 on malformed): scopes measurements + valuation to the catalog's songs linked to that artist account via song_artists (catalog_songs ∩ song_artists). The response echoes the applied filter as artist_account_id (null when unfiltered) so clients can verify the scope before rendering artist-labeled money. Per the docs contract in recoupable/docs#267. TDD: RED confirmed before GREEN for every unit; full suite 3,945 tests, tsc 0 new errors, lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe catalog measurements API moves from a flat query-parameter route to a catalog-scoped dynamic route. The handler and validator now accept a ChangesCatalog Measurements Refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Route as /api/catalogs/[catalogId]/measurements
participant Validator as validateGetCatalogMeasurementsQuery
participant Handler as getCatalogMeasurementsHandler
participant Supabase as Supabase RPC
Client->>Route: GET with catalogId, page, limit, artist_account_id
Route->>Handler: getCatalogMeasurementsHandler(request, catalogId)
Handler->>Validator: validateGetCatalogMeasurementsQuery(request, catalogId)
Validator->>Validator: validateAuthContext + schema parse
Validator-->>Handler: accountId, catalogId, page, limit, artist_account_id
Handler->>Supabase: selectCatalogMeasurementsAggregate
Handler->>Supabase: selectCatalogMeasurementsPage
Supabase-->>Handler: aggregate + page rows
Handler-->>Client: pagination, measured_song_count, total_streams, valuation
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 12 files
Confidence score: 2/5
- In
lib/supabase/catalog_songs/selectCatalogSongTitles.tsandlib/supabase/song_artists/selectSongArtistIsrcs.ts, pagination errors on later pages currently drop already-fetched results and return[], which is indistinguishable from a truly empty catalog and can silently undercount downstream data — preserve partial results or propagate an explicit error state before merging. - In
lib/supabase/song_measurements/selectAllSongMeasurements.ts, any page/chunk failure can causeGET /api/catalogs/measurementsto look like zero data/zero valuation, creating a concrete user-facing misreporting risk — return a typed failure (or throw) so callers can surface an error instead of treating it as empty data. - In
lib/catalog/getCatalogMeasurementsHandler.ts, artist-scoped valuation mixes artist-filtered stream totals with catalog-wide age, which can skew outputs for scoped views — derivecatalog_age_yearsfrom the same scoped song set before merge to keep calculations consistent. - Tests in
lib/supabase/song_measurements/__tests__/selectAllSongMeasurements.test.tsmiss coverage for the secondaryidsort assertion and for verifyingconsole.erroremission, so regressions in deterministic pagination/error observability may slip through — tighten these assertions to de-risk future changes.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/catalog/getCatalogMeasurementsHandler.ts">
<violation number="1" location="lib/catalog/getCatalogMeasurementsHandler.ts:48">
P2: Artist-scoped valuation can be skewed because the stream total is filtered to the artist while `catalog_age_years` still comes from the whole catalog. Consider resolving the earliest release date from the scoped song set (or a scoped album/source lookup) when `artist_account_id` is provided.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as API Client
participant Handler as getCatalogMeasurementsHandler
participant Validator as validateGetCatalogMeasurementsQuery
participant Auth as validateAuthContext
participant DBCatalog as selectAccountCatalog
participant Resolver as resolveCatalogSongsInScope
participant DBSongs as selectCatalogSongTitles
participant DBArtists as selectSongArtistIsrcs
participant DBMeas as selectAllSongMeasurements
Note over Handler: NEW: pagination + artist filter + response echo
Client->>Handler: GET /api/catalogs/measurements?catalogId=&artist_account_id=
Handler->>Validator: validate query params
Validator-->>Handler: {catalogId, artist_account_id} or 400
alt malformed artist_account_id (not uuid)
Handler-->>Client: 400 error
end
Handler->>Auth: validateAuthContext(request)
Auth-->>Handler: {accountId}
Handler->>DBCatalog: selectAccountCatalog({accountId, catalogId})
DBCatalog-->>Handler: catalog link or null
alt catalog not found or forbidden
Handler-->>Client: 404
end
Note over Handler: NEW: resolve scope (whole catalog vs artist filtered)
Handler->>Resolver: resolveCatalogSongsInScope({catalogId, artistAccountId})
Resolver->>DBSongs: CHANGED: selectCatalogSongTitles(catalogId)
Note over DBSongs: Paginates past 1,000-row default. Loops .range() until <1000 rows.
DBSongs-->>Resolver: all song {isrc, title} pairs
alt artistAccountId provided
Resolver->>DBArtists: NEW: selectSongArtistIsrcs(artistAccountId)
Note over DBArtists: Also paginates with .range() to exhaustion.
DBArtists-->>Resolver: ISRCs linked to artist
Note over Resolver: Intersect catalog songs with artist ISRCs
end
Resolver-->>Handler: songs in scope (possibly empty)
Note over Handler: CHANGED: empty scope skips measurement fetch
alt songs empty
Handler->>Handler: derive zero totals, zero valuation band
else songs non-empty
Handler->>DBMeas: NEW: selectAllSongMeasurements(songs, platform, metric)
Note over DBMeas: Chunks ISRC list (500 per chunk).<br/>Each chunk paginated with .range() to exhaustion.<br/>Order: captured_at descending, id descending.
DBMeas-->>Handler: all measurement rows
Handler->>Handler: latest per ISRC, total_streams, catalog_age_years, valuation band
end
Note over Handler: NEW: echo applied filter in response
Handler-->>Client: {measurements, total_streams, valuation, artist_account_id, catalog_age_years}
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
| const songs = await selectCatalogSongTitles(catalogId); | ||
| const songs = await resolveCatalogSongsInScope({ catalogId, artistAccountId }); |
There was a problem hiding this comment.
P2: Artist-scoped valuation can be skewed because the stream total is filtered to the artist while catalog_age_years still comes from the whole catalog. Consider resolving the earliest release date from the scoped song set (or a scoped album/source lookup) when artist_account_id is provided.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/getCatalogMeasurementsHandler.ts, line 48:
<comment>Artist-scoped valuation can be skewed because the stream total is filtered to the artist while `catalog_age_years` still comes from the whole catalog. Consider resolving the earliest release date from the scoped song set (or a scoped album/source lookup) when `artist_account_id` is provided.</comment>
<file context>
@@ -34,18 +38,18 @@ export async function getCatalogMeasurementsHandler(request: NextRequest): Promi
}
- const songs = await selectCatalogSongTitles(catalogId);
+ const songs = await resolveCatalogSongsInScope({ catalogId, artistAccountId });
const titles = new Map(songs.map(s => [s.isrc, s.title]));
const [rows, earliestReleaseDate] = await Promise.all([
</file context>
Preview verification — full authed pass ✅Preview deployment for Ground truth computed with read-only SQL against the shared prod DB (latest capture per ISRC over Authed checks (bearer for the owning test account)
Unauthed checks
Nothing pending: the test-account bearer authenticated against the preview (prod rejected it — previews use a different key salt — so the earlier prod 401 was salt, not expiry), so the full authed Done-when pass ran. 🤖 Generated with Claude Code |
…aller-paginated page (chat#1850) Amendment per the 2026-07-07 decision on chat#1850: the app-code .range() loop-to-exhaustion is rejected. The read path becomes: - aggregates (measured_song_count, total_streams -> valuation band) from the get_catalog_measurements_aggregate RPC: one SQL aggregate over the latest-per-ISRC subquery, whole scope, no row cap - the measurements array from the get_catalog_measurements_page RPC: page/limit params (default 1/50, limit max 100, 400 on invalid), rows sorted by playcount desc, same pagination envelope as GET /api/catalogs/songs - artist_account_id filter + response echo unchanged Both RPCs ship in recoupable/database#42 (applied to the shared project). Deletes the loop-based helpers (selectAllSongMeasurements, selectSongArtistIsrcs, selectCatalogSongTitles, resolveCatalogSongsInScope, latestMeasurementsPerIsrc) whose only consumer was this handler. TDD: RED confirmed before GREEN per unit; full suite 3,940 tests, tsc 200 pre-existing errors (0 new), lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
2 issues found across 19 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/catalog/getCatalogMeasurementsHandler.ts">
<violation number="1" location="lib/catalog/getCatalogMeasurementsHandler.ts:65">
P3: This handler now exceeds the repository's `lib/` 50-line function limit after adding pagination/aggregate response assembly. Consider extracting the measurement response-building or read orchestration into a small catalog-domain helper so the API handler stays within the documented boundary.</violation>
</file>
<file name="lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsPage.test.ts">
<violation number="1" location="lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsPage.test.ts:62">
P3: The `consoleError.mockRestore()` call in the body of the 'returns null on error' test runs only when the assertion passes. If it fails, the `console.error` spy leaks — other tests run with a silenced error logger. Since `restoreMocks` is not enabled in the vitest config and `clearAllMocks` doesn't restore spies, use `afterEach(() => consoleError.mockRestore())` or wrap the test body in `try/finally` for leak-proof cleanup.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -3,23 +3,28 @@ import { errorResponse } from "@/lib/networking/errorResponse"; | |||
| import { successResponse } from "@/lib/networking/successResponse"; | |||
There was a problem hiding this comment.
P3: This handler now exceeds the repository's lib/ 50-line function limit after adding pagination/aggregate response assembly. Consider extracting the measurement response-building or read orchestration into a small catalog-domain helper so the API handler stays within the documented boundary.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/catalog/getCatalogMeasurementsHandler.ts, line 65:
<comment>This handler now exceeds the repository's `lib/` 50-line function limit after adding pagination/aggregate response assembly. Consider extracting the measurement response-building or read orchestration into a small catalog-domain helper so the API handler stays within the documented boundary.</comment>
<file context>
@@ -38,36 +39,38 @@ export async function getCatalogMeasurementsHandler(request: NextRequest): Promi
return successResponse({
measurements,
+ pagination: {
+ total_count: aggregate.measuredSongCount,
+ page,
</file context>
| } as never); | ||
|
|
||
| expect(await selectCatalogMeasurementsPage({ catalogId, page: 1, limit: 50 })).toBeNull(); | ||
| consoleError.mockRestore(); |
There was a problem hiding this comment.
P3: The consoleError.mockRestore() call in the body of the 'returns null on error' test runs only when the assertion passes. If it fails, the console.error spy leaks — other tests run with a silenced error logger. Since restoreMocks is not enabled in the vitest config and clearAllMocks doesn't restore spies, use afterEach(() => consoleError.mockRestore()) or wrap the test body in try/finally for leak-proof cleanup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsPage.test.ts, line 62:
<comment>The `consoleError.mockRestore()` call in the body of the 'returns null on error' test runs only when the assertion passes. If it fails, the `console.error` spy leaks — other tests run with a silenced error logger. Since `restoreMocks` is not enabled in the vitest config and `clearAllMocks` doesn't restore spies, use `afterEach(() => consoleError.mockRestore())` or wrap the test body in `try/finally` for leak-proof cleanup.</comment>
<file context>
@@ -0,0 +1,64 @@
+ } as never);
+
+ expect(await selectCatalogMeasurementsPage({ catalogId, page: 1, limit: 50 })).toBeNull();
+ consoleError.mockRestore();
+ });
+});
</file context>
Preview verification — amended shape (
|
| Check | Expected | Actual |
|---|---|---|
missing catalogId |
400 | ✅ 400 catalogId parameter is required |
catalogId=not-a-uuid |
400 | ✅ 400 catalogId must be a valid UUID |
artist_account_id=not-a-uuid |
400 | ✅ 400 artist_account_id must be a valid UUID |
page=0 |
400 | ✅ 400 page must be positive |
limit=101 |
400 | ✅ 400 limit must be at most 100 |
limit=abc |
400 | ✅ 400 |
| valid params, no credentials | 401 | ✅ 401 |
OPTIONS |
200 | ✅ 200 |
Aggregate correctness (verified at the SQL layer — the exact functions the handler calls)
The RPCs from recoupable/database#42 (applied to the shared project) were executed directly against the live fixture catalog 7d3e5c35-1fac-4b88-aa90-e2e4a52dfe78; every scope matches the independent read-only SQL ground truth exactly:
| Scope | measured_song_count | total_streams | Match |
|---|---|---|---|
| whole catalog | 2,679 | 16,308,837,441 | ✅ exact |
Elvis Crespo b1814076… |
322 | 1,799,402,184 | ✅ exact |
September Mourning d85f9606… |
40 | 21,325,663 | ✅ exact |
Apache ebae4bb9… |
208 | 742,453,110 | ✅ exact |
J Alvarez 6f1d797f… |
608 | 6,717,937,120 | ✅ exact |
get_catalog_measurements_page(catalog, NULL, 3, 0) returns playcount-desc rows (top: USUYG1075006 717,012,221). The handler layer over these RPCs (pagination envelope math, echo field, 500s on RPC failure) is unit-tested: full suite 3,940 passing.
Pending an authed pass
The /tmp/test_bearer token for the owning test account has now expired (401 on the preview; it authenticated yesterday's loop-based pass). Pending until a fresh bearer is available: end-to-end authed 200s on this preview confirming the HTTP-level envelope (measurements page + pagination + measured_song_count + echo) against the numbers above. Given the RPCs are verified exact at the SQL layer and the handler is fully unit-tested, the remaining risk is wiring-level only.
🤖 Generated with Claude Code
…catalogId]/measurements (chat#1850) REST amendment on chat#1850: path for identity, query for modifiers — mirrors the research measurements family (app/api/research/albums/[id]/measurements). The route becomes a dynamic segment; the validator takes the path id (uuid, 400 on malformed; a catalogId smuggled into the query string is ignored) and the query carries only artist_account_id/page/limit. A missing id no longer routes (404 from Next), so the 'catalogId is required' 400 is gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Follow-up: endpoint renamed to
|
| Check | Expected | Actual |
|---|---|---|
old flat path /api/catalogs/measurements?catalogId=… |
404 (unrouted) | ✅ 404 |
/api/catalogs/not-a-uuid/measurements |
400 | ✅ 400 catalogId must be a valid UUID |
artist_account_id=not-a-uuid |
400 | ✅ 400 |
page=0 |
400 | ✅ 400 |
limit=101 |
400 | ✅ 400 |
| valid path, no credentials | 401 | ✅ 401 |
OPTIONS |
200 | ✅ 200 |
Suite 3,940 passing / tsc 0 new / lint clean. The authed end-to-end pass remains pending a fresh bearer (see the previous comment); aggregate correctness is already verified exact at the RPC/SQL layer.
🤖 Generated with Claude Code
There was a problem hiding this comment.
0 issues found across 6 files (changes from recent commits).
Requires human review: Auto-approval blocked by 3 unresolved issues from previous reviews.
Re-trigger cubic
| @@ -34,36 +43,39 @@ export async function getCatalogMeasurementsHandler(request: NextRequest): Promi | |||
| return authResult; | |||
| } | |||
There was a problem hiding this comment.
SRP
- actual: auth validation is in the handler
- required: auth validation is in validateGetCatalogMeasurementsQuery
There was a problem hiding this comment.
Fixed in cfed6de4 — validateGetCatalogMeasurementsQuery now owns auth (first) + input validation and returns { accountId, ...params }, matching the measurements-family validator convention (validateGetResearchTrackHistoricStatsRequest et al); the handler just destructures. One behavior note: an unauthenticated request with malformed params now returns 401 before the 400 param check (auth-first, same as the research siblings — the earlier verification tables showed 400 for those unauthed cases). Suite 3,940 passing, tsc 0 new, prettier clean.
…y (SRP) Review feedback on #763: the handler was doing auth validation itself. The validator now owns both auth and input validation (auth first), matching the measurements-family validator convention (validateGetResearchTrackHistoricStatsRequest et al) and returns { accountId, ...params }. Behavior note: an unauthenticated request with malformed params now 401s before the 400 param check, same as the research siblings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/catalog/validateGetCatalogMeasurementsQuery.ts (1)
6-29: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valuePrefer
z.uuid()herez.string({ message: ... }).uuid(...)is deprecated in Zod v4; switch to the top-level UUID helper (orz.guid()if you need the older permissive matcher).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/catalog/validateGetCatalogMeasurementsQuery.ts` around lines 6 - 29, The UUID validation in getCatalogMeasurementsQuerySchema still uses the deprecated z.string(...).uuid() pattern for catalogId and artist_account_id. Update those fields to use the top-level UUID helper in the schema while keeping the same required/custom error messaging behavior, and leave the page and limit validation logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/catalog/getCatalogMeasurementsHandler.ts`:
- Around line 46-58: The release-date lookup in getCatalogMeasurementsHandler is
not scoped the same way as the aggregate and page queries, so valuation band
calculations can mix artist-filtered streams with a catalog-wide age. Update the
Promise.all call to use an artist-aware release-date lookup (or pass
artistAccountId through to getCatalogEarliestReleaseDate) so
computeValuationBand receives earliestReleaseDate that matches the same artist
filter used by selectCatalogMeasurementsAggregate and
selectCatalogMeasurementsPage.
---
Nitpick comments:
In `@lib/catalog/validateGetCatalogMeasurementsQuery.ts`:
- Around line 6-29: The UUID validation in getCatalogMeasurementsQuerySchema
still uses the deprecated z.string(...).uuid() pattern for catalogId and
artist_account_id. Update those fields to use the top-level UUID helper in the
schema while keeping the same required/custom error messaging behavior, and
leave the page and limit validation logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b9d774f-efe3-4f85-aac4-630c4b0fb732
⛔ Files ignored due to path filters (7)
lib/catalog/__tests__/getCatalogMeasurementsHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/catalog/__tests__/latestMeasurementsPerIsrc.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsAggregate.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsPage.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**types/database.types.tsis excluded by none and included by none
📒 Files selected for processing (8)
app/api/catalogs/[catalogId]/measurements/route.tsapp/api/catalogs/measurements/route.tslib/catalog/getCatalogMeasurementsHandler.tslib/catalog/latestMeasurementsPerIsrc.tslib/catalog/validateGetCatalogMeasurementsQuery.tslib/supabase/catalog_songs/selectCatalogSongTitles.tslib/supabase/song_measurements/selectCatalogMeasurementsAggregate.tslib/supabase/song_measurements/selectCatalogMeasurementsPage.ts
💤 Files with no reviewable changes (3)
- lib/supabase/catalog_songs/selectCatalogSongTitles.ts
- app/api/catalogs/measurements/route.ts
- lib/catalog/latestMeasurementsPerIsrc.ts
| const [aggregate, measurements, earliestReleaseDate] = await Promise.all([ | ||
| selectCatalogMeasurementsAggregate({ catalogId, artistAccountId }), | ||
| selectCatalogMeasurementsPage({ catalogId, artistAccountId, page, limit }), | ||
| getCatalogEarliestReleaseDate(catalogId), | ||
| ]); | ||
| if (!aggregate || !measurements) { | ||
| return errorResponse("Internal server error", 500); | ||
| } | ||
|
|
||
| const { measurements, totalStreams } = latestMeasurementsPerIsrc(rows, titles); | ||
| const { valuation, catalogAgeYears } = computeValuationBand({ | ||
| totalStreams, | ||
| totalStreams: aggregate.totalStreams, | ||
| earliestReleaseDate, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether getCatalogEarliestReleaseDate supports artist scoping.
ast-grep run --pattern 'export async function getCatalogEarliestReleaseDate($$$) { $$$ }' --lang typescript lib
rg -n -A5 'function getCatalogEarliestReleaseDate' libRepository: recoupable/api
Length of output: 790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' lib/catalog/getCatalogMeasurementsHandler.ts
printf '\n---\n'
sed -n '1,220p' lib/catalog/getCatalogEarliestReleaseDate.ts
printf '\n---\n'
rg -n "getCatalogEarliestReleaseDate|earliestReleaseDate|catalog_age_years|valuation" lib/catalog lib -g '*.ts'Repository: recoupable/api
Length of output: 20186
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' lib/supabase/song_measurements/selectCatalogMeasurementsAggregate.ts
printf '\n---\n'
sed -n '1,220p' lib/supabase/song_measurements/selectCatalogMeasurementsPage.ts
printf '\n---\n'
sed -n '1,220p' lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts
printf '\n---\n'
sed -n '1,220p' lib/catalog/computeValuationBand.tsRepository: recoupable/api
Length of output: 13429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "artist_account_id|artistAccountId" lib/catalog lib/supabase marketing -g '*.ts'Repository: recoupable/api
Length of output: 8485
Scope the release-date lookup to the same artist filter as the measurement totals. selectCatalogMeasurementsAggregate/selectCatalogMeasurementsPage honor artist_account_id, but getCatalogEarliestReleaseDate(catalogId) does not. That mixes artist-scoped streams with a catalog-wide age when computing valuation and catalog_age_years, so filtered responses can show a misleading band. Pass artistAccountId through here or use an artist-scoped release-date helper.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/catalog/getCatalogMeasurementsHandler.ts` around lines 46 - 58, The
release-date lookup in getCatalogMeasurementsHandler is not scoped the same way
as the aggregate and page queries, so valuation band calculations can mix
artist-filtered streams with a catalog-wide age. Update the Promise.all call to
use an artist-aware release-date lookup (or pass artistAccountId through to
getCatalogEarliestReleaseDate) so computeValuationBand receives
earliestReleaseDate that matches the same artist filter used by
selectCatalogMeasurementsAggregate and selectCatalogMeasurementsPage.
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Requires human review: Auto-approval blocked by 3 unresolved issues from previous reviews.
Re-trigger cubic
Final preview verification —
|
| Scope | measured_song_count | total_streams | valuation.mid | echo | pagination |
|---|---|---|---|---|---|
| whole catalog | 2,679 ✅ | 16,308,837,441 ✅ | $132,456,300 ✅ | null ✅ |
50 rows, total_pages: 54 ✅ |
Elvis Crespo b1814076… |
322 ✅ | 1,799,402,184 ✅ | $14,614,295 ✅ | b1814076… ✅ |
— |
September Mourning d85f9606… (limit=1) |
40 ✅ | 21,325,663 ✅ | — | d85f9606… ✅ |
1 row, total_pages: 40 ✅ |
whole, page=2&limit=10 |
2,679 ✅ | — | — | — | 10 rows, total_pages: 268 ✅ |
Top row playcount-desc: USUYG1075006 717,012,221 — matches the RPC-layer check. The 1,000-row-cap regression is definitively gone at the HTTP layer (2,679 > 1,000 through the full route wiring).
Error semantics after the auth-first SRP refactor (review discussion_r3536632018)
| Check | Expected (new ordering) | Actual |
|---|---|---|
| bad path uuid, no auth | 401 (was 400 pre-refactor) | ✅ 401 |
| valid path, no auth | 401 | ✅ 401 |
| old flat route | 404 | ✅ 404 |
| OPTIONS | 200 | ✅ 200 |
| bad path uuid, authed | 400 catalogId must be a valid UUID |
✅ 400 |
bad artist_account_id, authed |
400 | ✅ 400 |
page=0 / limit=101, authed |
400 | ✅ 400 |
| unknown catalog, authed | 404 | ✅ 404 |
Docs ↔ API ↔ live reconciled: response fields match docs#267 (merged) field-for-field; no undocumented fields observed. Nothing pending — ready to merge.
🤖 Generated with Claude Code
1 participant
Implements the data plumbing v2: artist-scoped valuation api item of recoupable/chat#1850 — the contract is recoupable/docs#267 (merge that first).
What changed (same read path as api#757, two behaviors)
1. Fix: the silent 1,000-row cap (P1)
The endpoint computed
total_streams+ the valuation band over at most 1,000 rows — the Supabase default limit — with no error. Live repro on a 2,679-song catalog: exactly 1,000 measurements / 4.83B streams / $39.3M mid vs SQL ground truth 2,679 / 16.31B / ≈$132.5M (3.4× undervalued): #757 (comment)Now every read on the path paginates to exhaustion before dedupe/derivation:
lib/supabase/catalog_songs/selectCatalogSongTitles.ts—.range()loop (the tracklist itself capped at 1,000)lib/supabase/song_measurements/selectAllSongMeasurements.ts(new) — ISRC list chunked at 500 per query (boundedINclause / URL length) and each chunk.range()-paginated to exhaustion, with a stable secondary order for deterministic pageslib/supabase/song_artists/selectSongArtistIsrcs.ts(new) —.range()loop2. Feature: optional
artist_account_idquery param400on malformed) invalidateGetCatalogMeasurementsQuerysong_artists(catalog_songs ∩ song_artists, computed in the newlib/catalog/resolveCatalogSongsInScope.ts); absent → whole catalog (unchanged behavior)artist_account_idfield (uuid when scoped,nullwhen whole-catalog) so clients can verify the response scope before rendering artist-labeled numbers — pre-v2 deployments ignore the unknown param, and without the echo a client would show whole-catalog money under an artist label (the trust bug chat#1850 exists to kill)Verification
pnpm exec tsc --noEmit: 200 errors, all pre-existing in test files (0 new; 0 in touched files)pnpm lintclean[TEST] Full Roster Catalog (aggregate)fixture posted as a PR comment once the preview deploy is ReadyMerge order
recoupable/docs#267 (contract) → this PR → chat#1852 rework (hero consumes the filter + echo). Refs recoupable/chat#1850.
🤖 Generated with Claude Code
Summary by cubic
Reworks GET /api/catalogs/{catalogId}/measurements to return a paginated page of latest-per-ISRC Spotify playcounts with whole-scope totals and a SQL-derived valuation (no row cap). Adds optional artist scoping and moves
catalogIdto the path.New Features
artist_account_id(UUID; 400 on malformed) to scope results; echoed as UUID ornull.pageandlimit(default 1/50, max 100; 400 on invalid). Response includesmeasurements,pagination,measured_song_count,total_streams,valuation,artist_account_id, andcatalog_age_years.Refactors
catalogIdin the query.get_catalog_measurements_aggregatefor whole-scope totals andget_catalog_measurements_pagefor the page (sorted by playcount desc). Aggregates cover the entire scope regardless of page size.Written for commit cfed6de. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes